Add Redis-ready caching, sessions, presence, and event-state APIs#53
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces Redis-backed caching with in-memory fallback, session management via token hashing, cache-backed rate limiting, real-time presence tracking endpoints, and temporary event state storage. Authentication now validates active sessions before token verification and enforces role-based access controls. Changes
Sequence DiagramssequenceDiagram
actor Client
participant Server
participant SessionStore as Session Store
participant TokenValidator as Token Validator
Client->>Server: POST /api/orders<br/>(Authorization: Bearer token)
Server->>SessionStore: hasActiveSession(token)
alt Session Active
SessionStore-->>Server: true
Server->>TokenValidator: validateToken(token)
alt Token Valid
TokenValidator-->>Server: {userId, role, expiry}
Server->>Server: Check role-based permissions
alt Authorized
Server-->>Client: 200 OK + response
else Forbidden
Server-->>Client: 403 Forbidden
end
else Token Invalid
Server-->>Client: 401 Unauthorized
end
else Session Inactive
SessionStore-->>Server: false
Server-->>Client: 401 Unauthorized
end
sequenceDiagram
actor Client
participant Server
participant PresenceStore as Presence Store
participant Cache as Cache/Redis
Client->>Server: POST /api/presence/heartbeat<br/>{user, payload}
Server->>PresenceStore: heartbeat(user, payload)
PresenceStore->>Cache: set user presence data (TTL)
Cache-->>PresenceStore: stored
PresenceStore->>Cache: add user to presence set (TTL)
Cache-->>PresenceStore: added
PresenceStore-->>Server: success
Server-->>Client: 200 OK
Note over Client,Cache: Later query for active presence
Client->>Server: GET /api/presence/active?spotId=X
Server->>PresenceStore: listActive(spotId)
PresenceStore->>Cache: enumerate presence set<br/>(filter by spotId)
Cache-->>PresenceStore: [active users]
PresenceStore-->>Server: user list
Server-->>Client: 200 OK {users}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 CORS Access-Control-Allow-Methods header missing PUT, blocking browser requests to event-state endpoint
The PR adds a PUT /api/events/state/:eventKey endpoint at backend/server.js:391, but the Access-Control-Allow-Methods header in sendJson only lists 'GET,POST,DELETE,OPTIONS'. Since PUT is a non-simple HTTP method, browsers will issue a CORS preflight OPTIONS request, receive the allowed-methods list without PUT, and block the actual PUT request entirely.
Root Cause and Impact
The sendJson helper at backend/server.js:112 sets a static allowed-methods list:
'Access-Control-Allow-Methods': 'GET,POST,DELETE,OPTIONS',The new event-state write endpoint at backend/server.js:391 accepts PUT:
if ((method === 'PUT' || method === 'POST') && path.startsWith('/api/events/state/')) {Since PUT triggers a CORS preflight and the preflight response (routed through the OPTIONS handler at backend/server.js:146 which also calls sendJson) does not include PUT in the allowed methods, all browser-based PUT requests to this endpoint will fail. Only POST will work from browsers, making the PUT method dead code for any browser client.
(Refers to line 112)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const attempts = await cache.client.incr(attemptsKey); | ||
| if (attempts === 1) { | ||
| await cache.client.set(attemptsKey, String(attempts), { EX: Math.ceil(windowMs / 1000) }); | ||
| } |
There was a problem hiding this comment.
🟡 Race condition in recordFailure: SET after INCR can overwrite concurrent increments, resetting the rate-limit counter
When the first failed login attempt occurs, rateLimiter.recordFailure calls INCR then conditionally SET with a TTL when attempts === 1. Between these two operations (which are separate Redis round-trips), a concurrent request can also INCR the same key, and the subsequent SET from the first request overwrites the counter back to "1".
Detailed Explanation
At backend/cache.js:175-178:
const attempts = await cache.client.incr(attemptsKey);
if (attempts === 1) {
await cache.client.set(attemptsKey, String(attempts), { EX: Math.ceil(windowMs / 1000) });
}Race scenario with Redis:
- Request A:
INCR attemptsKey→ returns 1 - Request B:
INCR attemptsKey→ returns 2 - Request A:
SET attemptsKey "1" EX ttl→ overwrites value back to 1, losing Request B's increment - Request B:
attempts === 2, skips the SET branch
The counter is now "1" instead of "2", effectively discarding one failed attempt. Under sustained attack with concurrent requests, this can repeatedly reset the counter, allowing significantly more login attempts than maxAttempts before a block is triggered.
Fix: Use EXPIRE (or MemoryStore equivalent) to set the TTL without overwriting the value, e.g.:
if (attempts === 1) {
await cache.client.expire(attemptsKey, Math.ceil(windowMs / 1000));
}Prompt for agents
In backend/cache.js, rateLimiter.recordFailure (lines 175-178): replace the SET call that overwrites the INCR result with an EXPIRE-style call that only sets the TTL without changing the value. For the MemoryStore class, add an expire(key, seconds) method that sets this.expiries without touching this.values. For Redis, use the native EXPIRE command. This avoids the race where a concurrent INCR between the first request's INCR and SET gets lost when SET resets the value.
Was this helpful? React with 👍 or 👎 to provide feedback.
| import { cache, eventStateStore, getOrSetJsonCache, presenceStore, rateLimiter, sessionStore } from './cache.js'; | ||
| import "./env.js"; |
There was a problem hiding this comment.
🔴 cache.js reads env vars before dotenv.config() runs due to import ordering in server.js
The cache.js module is imported before env.js in server.js, so its top-level process.env reads execute before dotenv.config() loads the .env file. This means REDIS_URL, REDIS_KEY_PREFIX, CACHE_DEFAULT_TTL_SECONDS, and PRESENCE_TTL_SECONDS from a .env file will not be picked up by the cache module.
Root Cause and Impact
In backend/server.js:5-6:
import { cache, ... } from './cache.js';
import "./env.js";cache.js has top-level code at lines 3-6:
const REDIS_URL = process.env.REDIS_URL;
const REDIS_KEY_PREFIX = process.env.REDIS_KEY_PREFIX || 'brocode';
const CACHE_DEFAULT_TTL_SECONDS = Number(process.env.CACHE_DEFAULT_TTL_SECONDS || 60);
const PRESENCE_TTL_SECONDS = Number(process.env.PRESENCE_TTL_SECONDS || 60);And line 122 runs await createCacheClient() which uses REDIS_URL.
Since ES module imports execute in dependency order and cache.js includes a top-level await, it fully initializes before env.js calls dotenv.config(). If a user configures REDIS_URL=redis://localhost:6379 in their .env file (rather than as an actual environment variable), the cache module will see REDIS_URL as undefined and silently fall back to the in-memory store, ignoring the Redis configuration entirely.
| import { cache, eventStateStore, getOrSetJsonCache, presenceStore, rateLimiter, sessionStore } from './cache.js'; | |
| import "./env.js"; | |
| import "./env.js"; | |
| import { cache, eventStateStore, getOrSetJsonCache, presenceStore, rateLimiter, sessionStore } from './cache.js'; |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Motivation
Description
backend/cache.jsmodule that provides optional Redis connectivity with an automatic in-memory fallback plus helpers for JSON caching (getOrSetJsonCache), session storage (sessionStore), rate limiting (rateLimiter), presence (presenceStore), and temporary event-state (eventStateStore).backend/server.jsso login uses cache-backed rate limiting and writes an active session, auth-protected routes verify both token and active session, andPOST /api/auth/logoutinvalidates cached sessions.GET /api/catalogandGET /api/spots, and add presence and event-state endpoints:POST /api/presence/heartbeat,GET /api/presence/active,PUT|POST /api/events/state/:eventKey, andGET /api/events/state/:eventKey.backend/env.jswith Redis/cache-related env vars and updatebackend/README.mdto document the new features and environment settings (e.g.REDIS_URL,REDIS_KEY_PREFIX,CACHE_DEFAULT_TTL_SECONDS,PRESENCE_TTL_SECONDS,EVENT_STATE_DEFAULT_TTL_SECONDS).Testing
node --check backend/cache.js && node --check backend/server.js && node --check backend/env.js, which succeeded.PORT=4100 VITE_SUPABASE_URL=https://example.com VITE_SUPABASE_ANON_KEY=abcdefghijk node backend/server.js, but runtime start was blocked by a missing runtime dependency (dotenv) in this environment.npmbut the install failed due to registry access restrictions, so runtime integration tests requiring external packages could not be completed here.Codex Task
Summary by CodeRabbit
New Features
Security